Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add zoom & pan to charts #23183

Merged
merged 13 commits into from
Dec 12, 2024
Merged

Add zoom & pan to charts #23183

merged 13 commits into from
Dec 12, 2024

Conversation

MindFreeze
Copy link
Contributor

@MindFreeze MindFreeze commented Dec 6, 2024

Proposed change

Updates to the graph reset the chart so I had to stop updates while zoomed in. Shouldn't be an issue as the user is focusing on the data that is already there.

On mobile zoom & pan is done with touch.

On desktop

  • Pan by dragging with the mouse
  • Zoom:
    • ctrl + select a region of the chart to zoom in on
    • ctrl + scroll; an overlay is displayed on scroll like in embedded google maps

image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@MindFreeze MindFreeze added the WTH Issues & PRs generated from the "Month of What the Heck?" label Dec 6, 2024
@MindFreeze MindFreeze added the Noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Dec 6, 2024
@wendevlin
Copy link
Contributor

🎉
@MindFreeze We would need to add a reset zoom button somewhere

@MindFreeze
Copy link
Contributor Author

🎉 @MindFreeze We would need to add a reset zoom button somewhere

Indeed. I added double click to reset zoom for now but we need something more obvious for desktop. On mobile you can use your fingers

@MindFreeze MindFreeze marked this pull request as ready for review December 6, 2024 15:07
@MindFreeze MindFreeze added the Needs UX Pull requests requiring a review from the Home Assistant design team label Dec 6, 2024
@MindFreeze MindFreeze marked this pull request as draft December 11, 2024 08:45
@MindFreeze MindFreeze marked this pull request as ready for review December 11, 2024 14:24
@MindFreeze
Copy link
Contributor Author

We can add buttons later if we want but this is already very usable so we can merge it

@boern99
Copy link
Contributor

boern99 commented Dec 11, 2024

Very nice! One thing i noticed: If you try to zoom out on a chart using binary or text sensors bar chart: the chart gets hidden and does not come back. Behavior is on desktop and my mobile.

@bramkragten
Copy link
Member

ctrl+select also causes the same issue on timeline charts.

Also ctrl+select triggers the context menu for me... not an ideal UX

@MindFreeze
Copy link
Contributor Author

Also ctrl+select triggers the context menu for me... not an ideal UX

Now I remember that's why I used shift in the beginning

@MindFreeze
Copy link
Contributor Author

Looks like Google maps use the command button on Mac. Will change to that. They've made it work with ctrl as well but the hint says "⌘ + scroll"

@MindFreeze MindFreeze marked this pull request as draft December 11, 2024 17:28
@boern99
Copy link
Contributor

boern99 commented Dec 11, 2024

Maybe it would in a later step also be possible to sync time periods across multiple charts: If for example timeline charts and graph charts is loaded at the same time: Looks like the API could make this possible? chart.pan(delta, scales?, mode = 'none'): void (https://www.chartjs.org/chartjs-plugin-zoom/latest/guide/developers.html#imperative-zoom-pan-api)
On that base it perhaps would also be possible to recognize zoom out tries, even the chartjs-plugin has fully zoomed out, react on that and load a longer period of time?

@MindFreeze
Copy link
Contributor Author

Zooming out for more data is tricky because then you can't easily return to the initial state. We'd have to pollute the UI with a reset button. Panning to the left to load older data would be easier though

@MindFreeze
Copy link
Contributor Author

The text sensor chart will be fixed separately #23270

@MindFreeze MindFreeze marked this pull request as ready for review December 12, 2024 12:30
@bramkragten bramkragten merged commit 784b7e4 into dev Dec 12, 2024
15 checks passed
@bramkragten bramkragten deleted the chartjs-zoom branch December 12, 2024 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed Needs UX Pull requests requiring a review from the Home Assistant design team Noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) WTH Issues & PRs generated from the "Month of What the Heck?"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants